Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ci] isolate c_api_test library-loading from Python source tree #5761

Merged
merged 2 commits into from
Mar 7, 2023

Conversation

jameslamb
Copy link
Collaborator

@jameslamb jameslamb commented Mar 3, 2023

Contributes to #5061

This repo contains a few tests on LightGBM's C API which are run with pytest.

Today, they load lib_lightgbm.{dll/so} using a modified copy of the same library-loading code with ctypes used in the Python package. LightGBM's CI runs python setup.py bdist_wheel, then these C API tests rely on a lib_lightgbm.{dll/so} being available in python-package/compile/.

dll_path = [curr_path,
curr_path.parents[1],
curr_path.parents[1] / 'python-package' / 'lightgbm' / 'compile',
curr_path.parents[1] / 'python-package' / 'compile',
curr_path.parents[1] / 'lib']
if system() in ('Windows', 'Microsoft'):
dll_path.append(curr_path.parents[1] / 'python-package' / 'compile' / 'Release/')
dll_path.append(curr_path.parents[1] / 'python-package' / 'compile' / 'windows' / 'x64' / 'DLL')
dll_path.append(curr_path.parents[1] / 'Release')
dll_path.append(curr_path.parents[1] / 'windows' / 'x64' / 'DLL')
dll_path = [p / 'lib_lightgbm.dll' for p in dll_path]
else:
dll_path = [p / 'lib_lightgbm.so' for p in dll_path]

This PR proposes removing that reliance on the repo structure + exactly where lib_lightgbm.so copies get left behind after building Python wheels.

How does this contribute to #5061? The changes for that (in-progress in #5759) will involve building lightgbm wheels in an isolated build directory, which breaks the assumptions in c_api_test/test_.py that a lib_lightgbm will be left behind in the python-package/ directory.

How I tested this

Check that this works with the Python package installed.

pip uninstall --yes lightgbm
cd python-package/
python setup.py bdist_wheel
pip install dist/*.whl
cd ..
pytest tests/c_api_test

Check that it works without the Python package.

pip uninstall --yes lightgbm
rm -rf ./python-package/build
rm -rf ./python-package/compile
rm -rf ./python-package/dist
mkdir build
cd build
cmake ..
make -j3
cd ..
pytest tests/c_api_test

@jameslamb jameslamb changed the title [ci] isolate c_api_test library-loading from Python source tree WIP: [ci] isolate c_api_test library-loading from Python source tree Mar 3, 2023
@jameslamb jameslamb force-pushed the ci/simplify-c-api-tests branch from 9220128 to 92a8664 Compare March 3, 2023 03:01
@jameslamb jameslamb changed the title WIP: [ci] isolate c_api_test library-loading from Python source tree [ci] isolate c_api_test library-loading from Python source tree Mar 3, 2023
@jameslamb jameslamb marked this pull request as ready for review March 3, 2023 03:28
@jameslamb jameslamb requested review from StrikerRUS and jmoralez March 3, 2023 03:29
@jameslamb jameslamb mentioned this pull request Mar 3, 2023
60 tasks
@jameslamb
Copy link
Collaborator Author

@guolinke or @shiyu1994 can I please get a review on this this week? It's important for enabling the work to change the strategy for building Python wheels (#5061).

Thank you!

@jameslamb jameslamb merged commit 1585ee1 into master Mar 7, 2023
@jameslamb jameslamb deleted the ci/simplify-c-api-tests branch March 7, 2023 18:46
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity since it was closed.
To start a new related discussion, open a new issue at https://github.com/microsoft/LightGBM/issues
including a reference to this.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants